Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LEADER_NO_TIMEOUT_N #24581

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

LEADER_NO_TIMEOUT_N #24581

wants to merge 3 commits into from
+85 −2

Conversation

Mr-Asmodaeus
Copy link

@Mr-Asmodaeus Mr-Asmodaeus commented Nov 9, 2024 •
edited
Loading

Added a definable LEADER_NO_TIMEOUT_N to leader_key.c that allows disabling timeout for the initial N keys in the sequence (including leader)

Description

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Added a definable LEADER_NO_TIMEOUT_N to leader_key.c that allows dis…
64382bb 
…abling the timeout for the first N keystrokes in leader key sequences.
@Mr-Asmodaeus Mr-Asmodaeus marked this pull request as ready for review November 9, 2024 19:31
@Mr-Asmodaeus
Copy link
Author

Recreated after cleaning up my repo. Sorry for the earlier mess.

drashna
drashna reviewed Nov 9, 2024
View reviewed changes
docs/features/leader_key.md Outdated
Aditionally, you may want to disable the timeout for additional keystrokes after the leader key.
Add the following to your `config.h`:
```c
#define LEADER_NO_TIMEOUT_N <Number of keystrokes including leader>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is awkward and doesn't feel clear about what it does.

It sounds like LEADER_DISABLE_TIMEOUT_AFTER_KEYSTROKES_NUM might be more descriptive.

drashna
drashna requested changes Nov 9, 2024
View reviewed changes
Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since leader keys already have unit tests, any additional functionality should include additional tests to verify that the new functionality works as expected.

@zvecr zvecr changed the base branch from master to develop November 10, 2024 01:29
@zvecr
Copy link
Member

zvecr commented Nov 10, 2024

From the PR checklist,

all core PRs must now target develop branch, which will subsequently be merged back to master on the breaking changes timeline

I have updated the PR to reflect this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drashna drashna Awaiting requested review from drashna

Assignees
No one assigned
Labels
core documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Mr-Asmodaeus @zvecr @drashna